ChangesListSpecialPage: Implement buildMainQueryConds()

This also involved adding some default options to getDefaultOptions()
and a new method getDB().

Change-Id: I7389a72bfcf176480bfc36f9d6efc467e1a5e76a

diff --git a/includes/specialpage/ChangesListSpecialPage.php b/includes/specialpage/ChangesListSpecialPage.php
index 1d17394..c088adb 100644 (file)
--- a/includes/specialpage/ChangesListSpecialPage.php
+++ b/includes/specialpage/ChangesListSpecialPage.php
@@ -127,13 +127,20 @@ abstract class ChangesListSpecialPage extends SpecialPage {
 
        /**
         * Get a FormOptions object containing the default options. By default returns some basic options,
-        * you might not call parent method and discard them.
+        * you might want to not call parent method and discard them, or to override default values.
         *
         * @return FormOptions
         */
        public function getDefaultOptions() {
                $opts = new FormOptions();
 
+               $opts->add( 'hideminor', false );
+               $opts->add( 'hidebots', false );
+               $opts->add( 'hideanons', false );
+               $opts->add( 'hideliu', false );
+               $opts->add( 'hidepatrolled', false );
+               $opts->add( 'hidemyself', false );
+
                $opts->add( 'namespace', '', FormOptions::INTNULL );
                $opts->add( 'invert', false );
                $opts->add( 'associated', false );
@@ -185,12 +192,80 @@ abstract class ChangesListSpecialPage extends SpecialPage {
 
        /**
         * Return an array of conditions depending of options set in $opts
-        * @todo This should build some basic conditions here…
+        * @todo Whyyyy is this mutating $opts…
         *
         * @param FormOptions $opts
         * @return array
         */
-       abstract public function buildMainQueryConds( FormOptions $opts );
+       public function buildMainQueryConds( FormOptions $opts ) {
+               $dbr = $this->getDB();
+               $user = $this->getUser();
+               $conds = array();
+
+               // It makes no sense to hide both anons and logged-in users
+               // Where this occurs, force anons to be shown
+               $botsOnly = false;
+               if ( $opts['hideanons'] && $opts['hideliu'] ) {
+                       // Check if the user wants to show bots only
+                       if ( $opts['hidebots'] ) {
+                               $opts['hideanons'] = false;
+                       } else {
+                               $botsOnly = true;
+                       }
+               }
+
+               // Toggles
+               if ( $opts['hideminor'] ) {
+                       $conds['rc_minor'] = 0;
+               }
+               if ( $opts['hidebots'] ) {
+                       $conds['rc_bot'] = 0;
+               }
+               if ( $user->useRCPatrol() && $opts['hidepatrolled'] ) {
+                       $conds['rc_patrolled'] = 0;
+               }
+               if ( $botsOnly ) {
+                       $conds['rc_bot'] = 1;
+               } else {
+                       if ( $opts['hideliu'] ) {
+                               $conds[] = 'rc_user = 0';
+                       }
+                       if ( $opts['hideanons'] ) {
+                               $conds[] = 'rc_user != 0';
+                       }
+               }
+               if ( $opts['hidemyself'] ) {
+                       if ( $user->getId() ) {
+                               $conds[] = 'rc_user != ' . $dbr->addQuotes( $user->getId() );
+                       } else {
+                               $conds[] = 'rc_user_text != ' . $dbr->addQuotes( $user->getName() );
+                       }
+               }
+
+               // Namespace filtering
+               if ( $opts['namespace'] !== '' ) {
+                       $selectedNS = $dbr->addQuotes( $opts['namespace'] );
+                       $operator = $opts['invert'] ? '!=' : '=';
+                       $boolean = $opts['invert'] ? 'AND' : 'OR';
+
+                       // Namespace association (bug 2429)
+                       if ( !$opts['associated'] ) {
+                               $condition = "rc_namespace $operator $selectedNS";
+                       } else {
+                               // Also add the associated namespace
+                               $associatedNS = $dbr->addQuotes(
+                                       MWNamespace::getAssociated( $opts['namespace'] )
+                               );
+                               $condition = "(rc_namespace $operator $selectedNS "
+                                       . $boolean
+                                       . " rc_namespace $operator $associatedNS)";
+                       }
+
+                       $conds[] = $condition;
+               }
+
+               return $conds;
+       }
 
        /**
         * Process the query
@@ -202,6 +277,15 @@ abstract class ChangesListSpecialPage extends SpecialPage {
         */
        abstract public function doMainQuery( $conds, $opts );
 
+       /**
+        * Return a DatabaseBase object for reading
+        *
+        * @return DatabaseBase
+        */
+       protected function getDB() {
+               return wfGetDB( DB_SLAVE );
+       }
+
        /**
         * Send output to the OutputPage object, only called if not used feeds
         * @todo This should do most, if not all, of the outputting now done by subclasses

diff --git a/includes/specials/SpecialRecentchanges.php b/includes/specials/SpecialRecentchanges.php
index d0e6171..fdf8dcb 100644 (file)
--- a/includes/specials/SpecialRecentchanges.php
+++ b/includes/specials/SpecialRecentchanges.php
@@ -148,26 +148,14 @@ class SpecialRecentChanges extends ChangesListSpecialPage {
 
        /**
         * Return an array of conditions depending of options set in $opts
+        * @todo Whyyyy is this mutating $opts…
         *
         * @param FormOptions $opts
         * @return array
         */
        public function buildMainQueryConds( FormOptions $opts ) {
-               $dbr = wfGetDB( DB_SLAVE );
-               $conds = array();
-
-               # It makes no sense to hide both anons and logged-in users
-               # Where this occurs, force anons to be shown
-               $forcebot = false;
-               if ( $opts['hideanons'] && $opts['hideliu'] ) {
-                       # Check if the user wants to show bots only
-                       if ( $opts['hidebots'] ) {
-                               $opts['hideanons'] = false;
-                       } else {
-                               $forcebot = true;
-                               $opts['hidebots'] = false;
-                       }
-               }
+               $dbr = $this->getDB();
+               $conds = parent::buildMainQueryConds( $opts );
 
                // Calculate cutoff
                $cutoff_unixtime = time() - ( $opts['days'] * 86400 );
@@ -183,59 +171,6 @@ class SpecialRecentChanges extends ChangesListSpecialPage {
 
                $conds[] = 'rc_timestamp >= ' . $dbr->addQuotes( $cutoff );
 
-               $hidePatrol = $this->getUser()->useRCPatrol() && $opts['hidepatrolled'];
-               $hideLoggedInUsers = $opts['hideliu'] && !$forcebot;
-               $hideAnonymousUsers = $opts['hideanons'] && !$forcebot;
-
-               if ( $opts['hideminor'] ) {
-                       $conds['rc_minor'] = 0;
-               }
-               if ( $opts['hidebots'] ) {
-                       $conds['rc_bot'] = 0;
-               }
-               if ( $hidePatrol ) {
-                       $conds['rc_patrolled'] = 0;
-               }
-               if ( $forcebot ) {
-                       $conds['rc_bot'] = 1;
-               }
-               if ( $hideLoggedInUsers ) {
-                       $conds[] = 'rc_user = 0';
-               }
-               if ( $hideAnonymousUsers ) {
-                       $conds[] = 'rc_user != 0';
-               }
-
-               if ( $opts['hidemyself'] ) {
-                       if ( $this->getUser()->getId() ) {
-                               $conds[] = 'rc_user != ' . $dbr->addQuotes( $this->getUser()->getId() );
-                       } else {
-                               $conds[] = 'rc_user_text != ' . $dbr->addQuotes( $this->getUser()->getName() );
-                       }
-               }
-
-               # Namespace filtering
-               if ( $opts['namespace'] !== '' ) {
-                       $selectedNS = $dbr->addQuotes( $opts['namespace'] );
-                       $operator = $opts['invert'] ? '!=' : '=';
-                       $boolean = $opts['invert'] ? 'AND' : 'OR';
-
-                       # namespace association (bug 2429)
-                       if ( !$opts['associated'] ) {
-                               $condition = "rc_namespace $operator $selectedNS";
-                       } else {
-                               # Also add the associated namespace
-                               $associatedNS = $dbr->addQuotes(
-                                       MWNamespace::getAssociated( $opts['namespace'] )
-                               );
-                               $condition = "(rc_namespace $operator $selectedNS "
-                                       . $boolean
-                                       . " rc_namespace $operator $associatedNS)";
-                       }
-
-                       $conds[] = $condition;
-               }
-
                return $conds;
        }
 
@@ -252,7 +187,7 @@ class SpecialRecentChanges extends ChangesListSpecialPage {
                $query_options = array();
 
                $uid = $this->getUser()->getId();
-               $dbr = wfGetDB( DB_SLAVE );
+               $dbr = $this->getDB();
                $limit = $opts['limit'];
                $namespace = $opts['namespace'];
                $invert = $opts['invert'];
@@ -323,7 +258,7 @@ class SpecialRecentChanges extends ChangesListSpecialPage {
                $showWatcherCount = $wgRCShowWatchingUsers && $this->getUser()->getOption( 'shownumberswatching' );
                $watcherCache = array();
 
-               $dbr = wfGetDB( DB_SLAVE );
+               $dbr = $this->getDB();
 
                $counter = 1;
                $list = ChangesList::newFromContext( $this->getContext() );
@@ -537,7 +472,7 @@ class SpecialRecentChanges extends ChangesListSpecialPage {
         * @return string|bool
         */
        public function checkLastModified( $feedFormat ) {
-               $dbr = wfGetDB( DB_SLAVE );
+               $dbr = $this->getDB();
                $lastmod = $dbr->selectField( 'recentchanges', 'MAX(rc_timestamp)', false, __METHOD__ );
                if ( $feedFormat || !$this->getUser()->useRCPatrol() ) {
                        if ( $lastmod && $this->getOutput()->checkLastModified( $lastmod ) ) {

diff --git a/includes/specials/SpecialWatchlist.php b/includes/specials/SpecialWatchlist.php
index 519b6c2..a98447b 100644 (file)
--- a/includes/specials/SpecialWatchlist.php
+++ b/includes/specials/SpecialWatchlist.php
@@ -155,57 +155,14 @@ class SpecialWatchlist extends ChangesListSpecialPage {
         * @return array
         */
        public function buildMainQueryConds( FormOptions $opts ) {
-               $dbr = wfGetDB( DB_SLAVE, 'watchlist' );
-               $conds = array();
-               $user = $this->getUser();
+               $dbr = $this->getDB();
+               $conds = parent::buildMainQueryConds( $opts );
 
                // Calculate cutoff
                if ( $opts['days'] > 0 ) {
                        $conds[] = 'rc_timestamp > ' . $dbr->addQuotes( $dbr->timestamp( time() - intval( $opts['days'] * 86400 ) ) );
                }
 
-               # Toggles
-               if ( $opts['hidemyself'] ) {
-                       $conds[] = 'rc_user != ' . $user->getId();
-               }
-               if ( $opts['hidebots'] ) {
-                       $conds[] = 'rc_bot = 0';
-               }
-               if ( $opts['hideminor'] ) {
-                       $conds[] = 'rc_minor = 0';
-               }
-               if ( $opts['hideliu'] ) {
-                       $conds[] = 'rc_user = 0';
-               }
-               if ( $opts['hideanons'] ) {
-                       $conds[] = 'rc_user != 0';
-               }
-               if ( $user->useRCPatrol() && $opts['hidepatrolled'] ) {
-                       $conds[] = 'rc_patrolled != 1';
-               }
-
-               # Namespace filtering
-               if ( $opts['namespace'] !== '' ) {
-                       $selectedNS = $dbr->addQuotes( $opts['namespace'] );
-                       $operator = $opts['invert'] ? '!=' : '=';
-                       $boolean = $opts['invert'] ? 'AND' : 'OR';
-
-                       # namespace association (bug 2429)
-                       if ( !$opts['associated'] ) {
-                               $condition = "rc_namespace $operator $selectedNS";
-                       } else {
-                               # Also add the associated namespace
-                               $associatedNS = $dbr->addQuotes(
-                                       MWNamespace::getAssociated( $opts['namespace'] )
-                               );
-                               $condition = "(rc_namespace $operator $selectedNS "
-                                       . $boolean
-                                       . " rc_namespace $operator $associatedNS)";
-                       }
-
-                       $conds[] = $condition;
-               }
-
                return $conds;
        }
 
@@ -219,7 +176,7 @@ class SpecialWatchlist extends ChangesListSpecialPage {
        public function doMainQuery( $conds, $opts ) {
                global $wgShowUpdatedMarker;
 
-               $dbr = wfGetDB( DB_SLAVE, 'watchlist' );
+               $dbr = $this->getDB();
                $user = $this->getUser();
                # Toggle watchlist content (all recent edits or just the latest)
                if ( $opts['extended'] ) {
@@ -294,6 +251,15 @@ class SpecialWatchlist extends ChangesListSpecialPage {
                return $dbr->select( $tables, $fields, $conds, __METHOD__, $options, $join_conds );
        }
 
+       /**
+        * Return a DatabaseBase object for reading
+        *
+        * @return DatabaseBase
+        */
+       protected function getDB() {
+               return wfGetDB( DB_SLAVE, 'watchlist' );
+       }
+
        /**
         * Send output to the OutputPage object, only called if not used feeds
         *
@@ -303,7 +269,7 @@ class SpecialWatchlist extends ChangesListSpecialPage {
        public function webOutput( $rows, $opts ) {
                global $wgShowUpdatedMarker, $wgRCShowWatchingUsers;
 
-               $dbr = wfGetDB( DB_SLAVE, 'watchlist' );
+               $dbr = $this->getDB();
                $user = $this->getUser();
 
                $dbr->dataSeek( $rows, 0 );
@@ -481,7 +447,7 @@ class SpecialWatchlist extends ChangesListSpecialPage {
                $form = "";
                $user = $this->getUser();
 
-               $dbr = wfGetDB( DB_SLAVE, 'watchlist' );
+               $dbr = $this->getDB();
                $numItems = $this->countItems( $dbr );
 
                // Show watchlist header

diff --git a/tests/phpunit/includes/specials/SpecialRecentchangesTest.php b/tests/phpunit/includes/specials/SpecialRecentchangesTest.php
index b1ba152..c3d75aa 100644 (file)
--- a/tests/phpunit/includes/specials/SpecialRecentchangesTest.php
+++ b/tests/phpunit/includes/specials/SpecialRecentchangesTest.php
@@ -50,8 +50,7 @@ class SpecialRecentchangesTest extends MediaWikiTestCase {
                $this->assertConditions(
                        array( # expected
                                'rc_bot' => 0,
-                               #0 => "rc_timestamp >= '20110223000000'",
-                               1 => "rc_namespace = '0'",
+                               0 => "rc_namespace = '0'",
                        ),
                        array(
                                'namespace' => NS_MAIN,
@@ -63,9 +62,8 @@ class SpecialRecentchangesTest extends MediaWikiTestCase {
        public function testRcNsFilterInversion() {
                $this->assertConditions(
                        array( # expected
-                               #0 => "rc_timestamp >= '20110223000000'",
                                'rc_bot' => 0,
-                               1 => sprintf( "rc_namespace != '%s'", NS_MAIN ),
+                               0 => sprintf( "rc_namespace != '%s'", NS_MAIN ),
                        ),
                        array(
                                'namespace' => NS_MAIN,
@@ -82,9 +80,8 @@ class SpecialRecentchangesTest extends MediaWikiTestCase {
        public function testRcNsFilterAssociation( $ns1, $ns2 ) {
                $this->assertConditions(
                        array( # expected
-                               #0 => "rc_timestamp >= '20110223000000'",
                                'rc_bot' => 0,
-                               1 => sprintf( "(rc_namespace = '%s' OR rc_namespace = '%s')", $ns1, $ns2 ),
+                               0 => sprintf( "(rc_namespace = '%s' OR rc_namespace = '%s')", $ns1, $ns2 ),
                        ),
                        array(
                                'namespace' => $ns1,
@@ -101,9 +98,8 @@ class SpecialRecentchangesTest extends MediaWikiTestCase {
        public function testRcNsFilterAssociationWithInversion( $ns1, $ns2 ) {
                $this->assertConditions(
                        array( # expected
-                               #0 => "rc_timestamp >= '20110223000000'",
                                'rc_bot' => 0,
-                               1 => sprintf( "(rc_namespace != '%s' AND rc_namespace != '%s')", $ns1, $ns2 ),
+                               0 => sprintf( "(rc_namespace != '%s' AND rc_namespace != '%s')", $ns1, $ns2 ),
                        ),
                        array(
                                'namespace' => $ns1,